Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

simple changes #1219

Closed
wants to merge 5 commits into from
Closed

simple changes #1219

wants to merge 5 commits into from

Conversation

CPernet
Copy link

@CPernet CPernet commented Apr 22, 2024

add some windows subsystem info
fix some comma issue for DataCat example

Copy link

welcome bot commented Apr 22, 2024

Thank You Banner (Image: CC-BY license, The Turing Way Community, & Scriberia. Zenodo. https://doi.org/10.5281/zenodo.3332808) Wohoo! 🎉 Thanks for opening your first pull request to the DataLad Handbook! 😎 We really appreciate your time and effort to contribute to the project - you're amazing! 👏

@adswa
Copy link
Contributor

adswa commented Apr 22, 2024

@jsheunis could you review this? 🙏

@jsheunis
Copy link
Contributor

Thanks for tagging me, will review it 👍

Copy link
Contributor

@jsheunis jsheunis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @CPernet. Two points:

  • The addition of the commas in the catalog file are fine, those can be merged.
  • The additional note about WSL2 for windows installation is actually already covered in a note further down on that same page:
.. windows-wit:: Install DataLad using the Windows Subsystem 2 for Linux
   :name: ww-wsl2

   With the Windows Subsystem for Linux, you will be able to use a Unix system
   despite being on Windows.  You need to have a recent build of Windows in
   order to get WSL2 -- we do not recommend WSL1.

   You can find out how to install the Windows Subsystem for Linux at
   `docs.microsoft.com <https://learn.microsoft.com/en-us/windows/wsl/install>`_.
   Afterwards, proceed with your installation as described in the installation instructions
   for Linux.

So I would say the change to installation.rst is not necessary. Could you remove that commit from the PR?

@CPernet CPernet closed this Apr 24, 2024
@CPernet
Copy link
Author

CPernet commented Apr 24, 2024

pushing a clean one instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants